Add kaptcha verification to signup module#638
Open
vagisha wants to merge 4 commits into
Open
Conversation
* Added kaptchaText field to SignupForm * SignUpController.SignUpApiAction.execute() now checks the session kaptcha value, rejects mismatches, and clears the attribute on success * Populated error_message in the JSON response for invalid-email and validateSignupForm error paths so the wiki form can display them See ai/todos/active/TODO-LK-20260517_add-kaptcha-to-signup.md Co-Authored-By: Claude <noreply@anthropic.com>
* Extracted verifyCaptcha, parseAndValidateEmail, sendConfirmationEmail helpers; both actions now run the same sequence (captcha -> blank fields -> email format -> userExists -> send) * Added emailConfirm field to SignupForm; validateSignupForm now also checks blank + match * sendConfirmationEmail wraps the TempUser insert + sendEmail in a DbScope transaction so the row rolls back on send failure * SignUpApiAction now also runs EmailValidator.isValid (closes a real divergence from BeginAction) and no longer falls through to status=USER_ADDED after a sendEmail failure * Rewrote signupPage.jsp using <labkey:form>/<labkey:input> with Confirm Email and kaptcha sections See ai/todos/active/TODO-LK-20260517_add-kaptcha-to-signup.md Co-Authored-By: Claude <noreply@anthropic.com>
|
ERROR: A pull request from |
- Catch ConfigurationException from sendEmail (was only catching MessagingException) - Add status="ERROR" to API captcha-failure response for consistency with other response paths - Remove hardcoded "six characters" from kaptcha instruction text - Add aria-label to kaptcha input field - Use method reference in errorsToMessages stream Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds bot protection and improves correctness of the self-sign-up flow by introducing server-side captcha verification and aligning validation + email delivery handling across both the form POST and API signup paths.
Changes:
- Added server-side captcha verification and shared helper methods used by both
BeginActionandSignUpApiAction. - Added
emailConfirmfield and related validation to reduce email-entry mistakes. - Wrapped TempUser creation + confirmation email sending in a transaction to avoid persisting rows when email delivery fails; refactored signup JSP to use LabKey taglibs.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| signup/src/org/labkey/signup/signupPage.jsp | Reworked signup form markup to use <labkey:form> / <labkey:input>, added confirm-email field and captcha UI with reload behavior. |
| signup/src/org/labkey/signup/SignUpController.java | Added captcha verification + shared validation helpers, added confirm-email field support, and refactored email send + TempUser insert to be transactional. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
* SignUpApiAction.execute: add status="ERROR" to blank-field, bad-email,
and send-failure error paths (was inconsistent with captcha path)
* verifyCaptcha: use parameterized logging instead of string concatenation
* verifyCaptcha: fix typo in comment ("a a" -> "a")
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
labkey-jeckels
approved these changes
May 24, 2026
| { | ||
| errors.reject(ERROR_MSG, e.getMessage()); | ||
| } | ||
| return false; |
Contributor
There was a problem hiding this comment.
Do this with caution. Failing to commit() a transaction can leave things in an unwanted state. I think you'll be OK here because there's no wrapping transaction that will try to continue and later commit.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rationale
We need to add bot protection to the self-sign-up form.
Changes
SignUpApiActionandBeginActionemailConfirmfield requiring users to enter their email address twiceEmailValidator.isValidwas not called, and asendEmailfailure was falling through tostatus=USER_ADDEDsignupPage.jspto use<labkey:form>and<labkey:input>taglibsManual test plan
status=USER_EXISTSSee ai/todos/active/TODO-LK-20260517_add-kaptcha-to-signup.md
Co-Authored-By: Claude noreply@anthropic.com